Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change marco month_name.sql #123

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Change marco month_name.sql #123

merged 1 commit into from
Apr 5, 2024

Conversation

flixflop
Copy link
Contributor

@flixflop flixflop commented Apr 5, 2024

Context:

For newer versions of spark (>3.0), using the month flag 'L' in the databricks related macros leads to errors as documented here: https://github.com/calogica/dbt-date/issues/119.
In the Spark documentation (https://spark.apache.org/docs/latest/sql-ref-datetime-pattern.html) it is stated that
"'LLL': Short textual representation in the stand-alone form. It should be used to format/parse only months without any other date fields."
Whereas:
"'MMM': Short textual representation in the standard form. The month pattern should be a part of a date pattern not just a stand-alone month except locales where there is no difference between stand and stand-alone forms like in English."
Since the macro below is supposed to return the month name in a stand-alone form, the 'L' flag is probably also the preferred way to do this.

Changes:

I propose to change the flag 'L' with 'M' for the databricks related macro "spark__month_name" since this resolve the issue

* in marco spark__month_name we change the month identifier from L to M
@clausherther clausherther self-requested a review April 5, 2024 22:20
Copy link
Contributor

@clausherther clausherther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for fixing this!

@clausherther clausherther merged commit 9e3fa15 into calogica:main Apr 5, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants